Skip to content

[TT-14991] Fixed decode settings to fallback to global settings#901

Closed
lghiur wants to merge 1 commit intomasterfrom
TT-14991-fix-global-decode-settings
Closed

[TT-14991] Fixed decode settings to fallback to global settings#901
lghiur wants to merge 1 commit intomasterfrom
TT-14991-fix-global-decode-settings

Conversation

@lghiur
Copy link
Copy Markdown
Collaborator

@lghiur lghiur commented Oct 9, 2025

Addresses https://tyktech.atlassian.net/browse/TT-14991

Description

This fix will make it so that if a decode setting (raw_request_decoded or raw_response_decoded) is false or not defined at the pump level, then it will fallback to the global settings.

Related Issue

Motivation and Context

Customers had to enable decoding settings for each and every pump they wanted this behaviour in.

How This Has Been Tested

  1. Unit tests
  2. Manually: I have configured a CSV pump, and then tested different variations of the values as the ticket describes.

Example of config

"raw_request_decoded": true,
"raw_response_decoded": false,
"pumps": {
      "csv": {
        "type": "csv",
        "raw_request_decoded": false,
        "raw_response_decoded": false,
        "meta": {
          "csv_dir": "./reports"
        }
      }
    },

Trigger variaous requests to the API, and observe the files created under /reports folder and you can check the results.

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch (right side). If pulling from your own
    fork, don't request your master!
  • Make sure you are making a pull request against the master branch (left side). Also, you should start
    your branch off our latest master.
  • My change requires a change to the documentation.
    • If you've changed APIs, describe what needs to be updated in the documentation.
  • I have updated the documentation accordingly.
  • Modules and vendor dependencies have been updated; run go mod tidy && go mod vendor
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Check your code additions will not fail linting checks:
    • go fmt -s
    • go vet

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Oct 9, 2025

🔍 Code Analysis Results

Pull Request Analysis: TT-14991 Fixed decode settings to fallback to global settings

This pull request addresses a bug in how decoding settings for raw requests and responses are handled, ensuring that pump-specific settings correctly fall back to global configurations.


1. Change Impact Analysis

What this PR accomplishes

The primary goal of this PR is to fix the configuration inheritance logic for raw_request_decoded and raw_response_decoded settings. Previously, if these settings were disabled or not present at the individual pump level, the global settings were ignored. This change ensures that if a pump-level setting is false, the system falls back to the corresponding global setting. This makes the configuration more intuitive and flexible, allowing a global default to be overridden only when explicitly configured at the pump level.

Key Technical Changes

The changes are concentrated in main.go with corresponding tests in main_test.go.

  1. Refactored Logic into Helper Functions: The data processing logic within the filterData function has been modularized and extracted into smaller, single-responsibility functions:

    • getDecodingSettings(pump pumps.Pump): This new function encapsulates the core logic of the fix. It determines the effective decoding settings by combining the pump-specific and global configurations using a logical OR. The effective setting is true if either the pump-level or the global-level setting is true.
    • decodeBase64Data(...): The logic for Base64 decoding the raw request and response has been moved into this dedicated function, improving code clarity.
    • trimRawData(...): The logic for trimming analytics records to a maximum size has also been extracted.
  2. Updated filterData Function: The main filterData function is now cleaner and more readable. It calls the new helper functions to determine settings, trim data, and perform decoding, rather than containing all the logic inline.

  3. Comprehensive Unit Tests: A new test suite, TestGetDecodingSettings, has been added in main_test.go. It exhaustively tests all possible combinations of pump-level and global-level decoding settings, verifying that the new fallback logic works as expected in every scenario.

Affected System Components

  • Configuration Handling: The change directly impacts how configurations are interpreted, specifically for analytics data decoding.
  • Data Processing Pipeline: The filterData function, a key part of the analytics processing pipeline, is modified. This function is responsible for preparing analytics records before they are sent to the configured pumps (e.g., CSV, Elasticsearch, Splunk).

2. Architecture Visualization

The following flowchart visualizes the updated logic within the filterData function, highlighting the new decision-making process for applying decoding settings.

flowchart TD
    subgraph "filterData Function"
        A[Analytics Record Received] --> B{Get Decoding Settings};
        B --> C{Should Trim Data?};
        C -- Yes --> D[trimRawData];
        C -- No --> E;
        D --> E{Apply Filters};
        E --> F{Remove Ignored Fields};
        F --> G{Perform Base64 Decoding?};
        G -- Yes --> H[decodeBase64Data];
        G -- No --> I[Send to Pump];
        H --> I;
    end

    subgraph "getDecodingSettings Logic"
        B1[Start] --> B2{Pump raw_request_decoded is true?};
        B2 -- Yes --> B3[Use true for Request Decoding];
        B2 -- No --> B4{Global DecodeRawRequest is true?};
        B4 -- Yes --> B3;
        B4 -- No --> B5[Use false for Request Decoding];

        B6[Start] --> B7{Pump raw_response_decoded is true?};
        B7 -- Yes --> B8[Use true for Response Decoding];
        B7 -- No --> B9{Global DecodeRawResponse is true?};
        B9 -- Yes --> B8;
        B9 -- No --> B10[Use false for Response Decoding];
    end

    style getDecodingSettings Logic fill:#f9f,stroke:#333,stroke-width:2px

Loading

This diagram illustrates how an analytics record is processed. The key change is in the "Get Decoding Settings" step, which now correctly combines pump and global settings to decide whether the decodeBase64Data step should be executed. The refactoring also clarifies the data trimming, filtering, and field removal steps in the pipeline.


Powered by Visor from Probelabs

Last updated: 2025-10-09T08:23:37.979Z | Triggered by: opened | Commit: 4cf6f09

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Oct 9, 2025

🔍 Code Analysis Results

Security Issues (1)

Severity Location Issue
🟡 Warning main.go:355
The configuration fallback logic may cause unintentional decoding of sensitive data. If a pump-level decoding setting is explicitly `false` but the global setting is `true`, the data will still be decoded, potentially exposing sensitive information.
💡 SuggestionModify the configuration logic to distinguish between an unset value and an explicit `false`. This can be achieved by using a pointer to a boolean (`*bool`) for the decoding settings in the pump configuration. The `getDecodingSettings` function should then check if the pump-level setting is `nil`. If it is, fall back to the global setting; otherwise, use the pump-level value.
🔧 Suggested Fix
func getDecodingSettings(pump pumps.Pump) (decodingRequest, decodingResponse bool) {
    // Example for decodingRequest
	if pump.GetDecodedRequest() != nil {
		decodingRequest = *pump.GetDecodedRequest()
	} else {
		decodingRequest = SystemConfig.DecodeRawRequest
	}
// Apply similar logic for decodingResponse
if pump.GetDecodedResponse() != nil {
	decodingResponse = *pump.GetDecodedResponse()
} else {
	decodingResponse = SystemConfig.DecodeRawResponse
}

return decodingRequest, decodingResponse

}

Performance Issues (3)

Severity Location Issue
🟠 Error main.go:381
The code calls the method `Clone()` on `analytics.AnalyticsRecord`, but this method does not appear to be defined in the repository or its dependencies based on the provided context. This will cause a compilation error.
💡 SuggestionEnsure the `Clone()` method implementation is included in this pull request or that the relevant dependency providing it has been updated. The PR cannot be merged in its current state.
🟠 Error system:0
Global failure condition met
🟡 Warning main.go:381
The introduction of `Clone()` inside this loop creates a new `AnalyticsRecord` object for every record in every batch, for every pump. This will significantly increase memory allocations and garbage collector pressure in a performance-critical path, potentially degrading throughput.
💡 SuggestionWhile cloning is necessary for correctness to prevent data corruption between pumps, its performance impact should be acknowledged and monitored. Consider a lazy-cloning approach where the record is only copied if a modification (like trimming or decoding) is actually required for the specific pump configuration. This would avoid allocations for pumps that do not alter the record.

✅ Quality Check Passed

No quality issues found – changes LGTM.

✅ Style Check Passed

No style issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-10-09T08:23:38.797Z | Triggered by: opened | Commit: 4cf6f09

@lghiur lghiur closed this Oct 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant